feat(sunburst): add innerRadiusRatio, renderRootNode prop to control size of inner circle #1794
Conversation
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit 0c1643e:
|
adamberro
changed the title
|
Hi @plouc, anything we can do to get the review process started? Thank you! |
|
I really like this feature and need it on my project too! |
|
|
||
| const maxDepth = Math.max(...sortedNodes.map(n => n.depth)) | ||
|
|
||
| const scale = d3ScaleRadial().domain([0, maxDepth]).range([innerRadiusOffset, radius]) |
There was a problem hiding this comment.
using d3 scaleLinear instead of scaleRadial would render better in my opinion
There was a problem hiding this comment.
scaleLinear looks quite different from the original implementation:
Maybe we could update the sunburst props to include setters for the arc inner/outer radii. I'm not sure if this reaches a happy medium between customizability and ease of use, but my initial thought for the api would be something like:
type GetInnerRadius = (currentNode: HierarchyRectangularNode<RawDatum>, nodes: HierarchyRectangularNode<RawDatum>[]) => number
@plouc Thoughts on this approach?
|
Hi @plouc, just checking in on whether we might be able to merge this. I'm happy to make any changes as needed. Thanks! |
| @@ -45,6 +45,11 @@ export const useArcCentersTransition = <Datum extends DatumWithArc, ExtraProps = | |||
| mode: ArcTransitionMode = 'innerRadius', | |||
| extra?: TransitionExtra<Datum, ExtraProps> | |||
| ) => { | |||
| // center label of root node | |||
There was a problem hiding this comment.
This centers the root most label. Not sure if there is a more proper way to handle this, but I don't think it was accounted for originally
.
|
@adamberro, sorry for the late reply, I'll try to run this branch locally and review the PR in the upcoming weeks. |
|
Also looking forward to this new feature 😄 |
|
This pull request has been automatically marked as stale. If this pull request is still relevant, please leave any comment (for example, "bump"), and we'll keep it open. We are sorry that we haven't been able to prioritize reviewing it yet. Your contribution is very much appreciated. |
|
bump |
|
bump @plouc this is a great feature. Can it be reviewed and merged? @adamberro Looks like your MR is stale. would you mind resubmitting with the latest changes? |
|
Hi! I hope this MR is able to merge. I would like this changes. Thank you |
|
This pull request has been automatically marked as stale. If this pull request is still relevant, please leave any comment (for example, "bump"), and we'll keep it open. We are sorry that we haven't been able to prioritize reviewing it yet. Your contribution is very much appreciated. |
|
Closing this pull request after a prolonged period of inactivity. If this issue is still present in the latest release, please ask for this pull request to be reopened. Thank you! |
innerRadiusprop to change the size of the sunburst chart's inner circle.Fixes #1649
Please let me know if this approach works or if it would be more desirable to give more flexibility such as the option to configure the type of d3 scale, or if there are any other issues or anything else is missing.